Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: refactor test-http(s)-set-timeout-server #13935

Closed
wants to merge 1 commit into from
Closed

test: refactor test-http(s)-set-timeout-server #13935

wants to merge 1 commit into from

Conversation

aqrln
Copy link
Contributor

@aqrln aqrln commented Jun 26, 2017

  • Make changes to test-https-set-timeout-server to resolve
    inconsistencies with its http counterpart:

    • Apply the changes analogous to those in test: refactor test-http-set-timeout-server #13802 to the https test.
    • Add a missing common.mustCall() wrapper.
    • Make small stylistic changes (e.g., remove unnecessary line breaks
      in comments) to make it visually consistent with the http test.

  • Use arrow functions.

Refs: #13802
Refs: #13625
Refs: #13822
Fixes: #13588

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@aqrln
test: refactor test-http(s)-set-timeout-server
9770ebb 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in nodejsGH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

Refs: nodejs#13802
Refs: nodejs#13625
Refs: nodejs#13822
Fixes: nodejs#13588
@aqrln aqrln added http Issues or PRs related to the http subsystem. https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests. labels Jun 26, 2017
@aqrln
Copy link
Contributor Author

aqrln commented Jun 26, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/8832/

@targos
Copy link
Member

targos commented Jul 3, 2017

ping @aqrln

@refack
Copy link
Contributor

refack commented Jul 3, 2017

ping @aqrln

[acting as @aqrln's vacation responder] He's away for a few days — #14033 (comment)

@targos
Copy link
Member

targos commented Jul 3, 2017

OK thank you. Then anyone feel free to re-run CI and land :)

@refack
Copy link
Contributor

refack commented Jul 3, 2017

CI: https://ci.nodejs.org/job/node-test-commit/10922/

refack pushed a commit to refack/node that referenced this pull request Jul 3, 2017
@aqrln @refack
test: refactor test-http(s)-set-timeout-server
932ee65 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in nodejsGH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: nodejs#13935
Fixes: nodejs#13588
Refs: nodejs#13802
Refs: nodejs#13625
Refs: nodejs#13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@refack
Copy link
Contributor

refack commented Jul 3, 2017

Landed in 932ee65

@refack refack closed this Jul 3, 2017
addaleax pushed a commit to addaleax/node that referenced this pull request Jul 3, 2017
@aqrln @addaleax
test: refactor test-http(s)-set-timeout-server
c2c7eb1 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in nodejsGH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: nodejs#13935
Fixes: nodejs#13588
Refs: nodejs#13802
Refs: nodejs#13625
Refs: nodejs#13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax mentioned this pull request Jul 3, 2017
Merged
@aqrln aqrln deleted the refactor-test-http-set-timeout-server branch July 6, 2017 14:10
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@aqrln @addaleax
test: refactor test-http(s)-set-timeout-server
92656e5 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in GH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: #13935
Fixes: #13588
Refs: #13802
Refs: #13625
Refs: #13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@aqrln @addaleax
test: refactor test-http(s)-set-timeout-server
534ae44 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in GH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: #13935
Fixes: #13588
Refs: #13802
Refs: #13625
Refs: #13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
@aqrln @Fishrock123
test: refactor test-http(s)-set-timeout-server
16a2f68 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in GH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: #13935
Fixes: #13588
Refs: #13802
Refs: #13625
Refs: #13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@aqrln @MylesBorins
test: refactor test-http(s)-set-timeout-server
c5a427f 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in GH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: #13935
Fixes: #13588
Refs: #13802
Refs: #13625
Refs: #13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 15, 2017
@aqrln @MylesBorins
test: refactor test-http(s)-set-timeout-server
6b3f8da 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in GH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: #13935
Fixes: #13588
Refs: #13802
Refs: #13625
Refs: #13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
@aqrln @MylesBorins
test: refactor test-http(s)-set-timeout-server
346f199 
* Make changes to `test-https-set-timeout-server` to resolve
  inconsistencies with its http counterpart:

  - Apply the changes analogous to those in GH-13802 to the https test.
  - Add a missing `common.mustCall()` wrapper.
  - Make small stylistic changes (e.g., remove unnecessary line breaks
    in comments) to make it visually consistent with the http test.

* Use arrow functions.

PR-URL: #13935
Fixes: #13588
Refs: #13802
Refs: #13625
Refs: #13822
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2017
Merged
This was referenced Sep 5, 2017
Closed
Closed
This was referenced Sep 6, 2017
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hiroppy hiroppy hiroppy approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
http Issues or PRs related to the http subsystem. https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test: test-http(s)-set-timeout-server.js tests should be more similar
8 participants
@aqrln @targos @refack @jasnell @lpinca @cjihrig @hiroppy @MylesBorins